Skip to content

Support returning default index_options for semantic_text fields when include_defaults is true #129967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
Jun 26, 2025
Merged

Support returning default index_options for semantic_text fields when include_defaults is true #129967

merged 13 commits into from
Jun 26, 2025

Conversation

@kderusso
Copy link
Member

@kderusso kderusso commented Jun 24, 2025

This PR fixes an issue from #119967 where the default index_options were not displayed when include_defaults is set to true.

Part of this fix requires serializing the model_settings earlier so they're accessible when we're performing serialization checks for index_options.

Here is an example of how to test this:

PUT my-dense-semantic-index/
{
  "mappings": {
    "properties": {
      "inference_field": {
        "type": "semantic_text",
        "inference_id": "my-e5-model"
        }
      }
    }
  }
}

// Should not return index_options
GET my-dense-semantic-index/_mapping

// Returns BBQ hnsw default index_options
GET my-dense-semantic-index/_mapping/field/inference_field?include_defaults

@kderusso kderusso added v9.2.0 >bug :Search Relevance/Search Catch all for Search Relevance and removed v9.1.0 labels Jun 25, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jun 25, 2025

Hi @kderusso, I've created a changelog YAML for you.

@kderusso kderusso marked this pull request as ready for review June 25, 2025 12:44
@kderusso kderusso requested review from Mikep86 and jimczi June 25, 2025 12:44
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Jun 25, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jun 25, 2025

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@kderusso kderusso requested a review from a team June 25, 2025 19:07
Mikep86
Mikep86 reviewed Jun 25, 2025
View reviewed changes
Copy link
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks good overall, I have some follow-up questions about null serialization though.

@Mikep86
Copy link
Contributor

Mikep86 commented Jun 25, 2025

Also, can we add a test showing that index_options provided by the user are always serialized?

@kderusso
Copy link
Member Author

kderusso commented Jun 25, 2025

Also, can we add a test showing that index_options provided by the user are always serialized?

@Mikep86 Please see the tests starting here and let me know if you believe there's a gap.

@Mikep86
Copy link
Contributor

Mikep86 commented Jun 25, 2025

@kderusso This test doesn't cover what I had in mind. I think it would be good to check that if we provide a mapping like:

PUT my-index
{
  "mappings": {
    "properties": {
      "dense_inference": {
        "type": "semantic_text",
        "inference_id": "some_dense_inference_id",
        "index_options": {
          <BBQ index options that match defaults>
        }
      }
    }
  }
}

The index_options are always serialized, even though they match defaults, because they are user-provided. This behavior differs from other field types, so good to capture in a test.

@kderusso kderusso requested a review from Mikep86 June 26, 2025 12:29
Mikep86
Mikep86 approved these changes Jun 26, 2025
View reviewed changes
Copy link
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the changes!

@kderusso kderusso merged commit 81a6ead into elastic:main Jun 26, 2025
32 checks passed
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Jun 27, 2025
@kderusso @smalyshev
Support returning default index_options for semantic_text fields when…
cabc063 
… include_defaults is true (elastic#129967)
@Mikep86 Mikep86 mentioned this pull request Jul 2, 2025
Merged
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
@kderusso @mridula-s109
Support returning default index_options for semantic_text fields when…
e480afd 
… include_defaults is true (elastic#129967)
@Mikep86 Mikep86 mentioned this pull request Jul 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mikep86 Mikep86 Mikep86 approved these changes

@jimczi jimczi Awaiting requested review from jimczi

Assignees

No one assigned

Labels

>bug :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kderusso @elasticsearchmachine @Mikep86